Skip to content

Conversation

pietroalbini
Copy link
Member

The git integration in build_helper hardcoded rust-lang/rust as the parent GitHub repository, and master as the branch name. This works great for rust-lang/rust, but causes problems in downstream forks like Ferrocene whenever those functions are invoked (like ./x fmt).

In src/stage0.json there was already a configuration key for the name of the nightly branch, but it wasn't used by build_helper. This PR adds the github_repository key to the file, and requires both values to be passed to build_helper whenever a git function is called. This will allow downstream forks to tweak the values.

The git integration in build_helper hardcoded `rust-lang/rust` as the
parent GitHub repository, and `master` as the branch name. This works
great for `rust-lang/rust`, but causes problems in downstream forks like
Ferrocene whenever those functions are invoked (like `./x fmt`).

In `src/stage0.json` there was already a configuration key for the name
of the nightly branch, but it wasn't used by build_helper. This commit
adds the `github_repository` key to the file, and requires both values
to be passed to build_helper whenever a git function is called. This
will allow downstream forks to tweak the values.
@rustbot
Copy link
Collaborator

rustbot commented Oct 24, 2023

r? @albertlarsan68

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-release Relevant to the release subteam, which will review and decide on the PR/issue. labels Oct 24, 2023
@rustbot
Copy link
Collaborator

rustbot commented Oct 24, 2023

This PR modifies src/bootstrap/src/core/config. If appropriate, please also update CONFIG_CHANGE_HISTORY in src/bootstrap/src/lib.rs and change-id in config.example.toml.

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

@albertlarsan68
Copy link
Member

This is a good idea, thanks!
Can you update config.example.toml with the default values and explanation about the new config?

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Oct 24, 2023

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

This PR modifies src/bootstrap/src/core/config. If appropriate, please also update CONFIG_CHANGE_HISTORY in src/bootstrap/src/lib.rs and change-id in config.example.toml.

@pietroalbini
Copy link
Member Author

Can you update config.example.toml with the default values and explanation about the new config?

Right now the configuration is not in config.toml/config.example.toml, but in src/stage0.json, for two reasons:

  • nightly_branch was already there, so I added github_repository alongside it.
  • This is not really a configuration the user should set, but a configuration of the repository/fork itself. It should work without users having to set any config value when cloning Ferrocene (or any other fork of the compiler). I agree src/stage0.json is suboptimal for this, but there is no other place like that right now.

@rust-log-analyzer

This comment has been minimized.

@albertlarsan68
Copy link
Member

My bad, I was thinking it was an option in config.toml.
I'll review this when I have the time

Copy link
Member

@albertlarsan68 albertlarsan68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I would prefer if github_repository was renamed to git_repository.
r=me with the change

@albertlarsan68 albertlarsan68 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2023
@pietroalbini
Copy link
Member Author

@bors r=albertlarsan68

@bors
Copy link
Collaborator

bors commented Nov 6, 2023

📌 Commit 580fa0c has been approved by albertlarsan68

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 6, 2023
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Nov 6, 2023

⌛ Testing commit 580fa0c with merge 2f0d41a...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 6, 2023
…lbertlarsan68

Allow configuring the parent GitHub repository

The git integration in build_helper hardcoded `rust-lang/rust` as the parent GitHub repository, and `master` as the branch name. This works great for `rust-lang/rust`, but causes problems in downstream forks like Ferrocene whenever those functions are invoked (like `./x fmt`).

In `src/stage0.json` there was already a configuration key for the name of the nightly branch, but it wasn't used by build_helper. This PR adds the `github_repository` key to the file, and requires both values to be passed to build_helper whenever a git function is called. This will allow downstream forks to tweak the values.
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Nov 6, 2023

💔 Test failed - checks-actions

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 6, 2023
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 6, 2023
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this pull request Nov 9, 2023
…=albertlarsan68

Allow configuring the parent GitHub repository

The git integration in build_helper hardcoded `rust-lang/rust` as the parent GitHub repository, and `master` as the branch name. This works great for `rust-lang/rust`, but causes problems in downstream forks like Ferrocene whenever those functions are invoked (like `./x fmt`).

In `src/stage0.json` there was already a configuration key for the name of the nightly branch, but it wasn't used by build_helper. This PR adds the `github_repository` key to the file, and requires both values to be passed to build_helper whenever a git function is called. This will allow downstream forks to tweak the values.
@pietroalbini
Copy link
Member Author

@bors r=albertlarsan68

@bors
Copy link
Collaborator

bors commented Nov 9, 2023

📌 Commit 488dd9b has been approved by albertlarsan68

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2023
@bors
Copy link
Collaborator

bors commented Nov 9, 2023

⌛ Testing commit 488dd9b with merge 4c8862b...

@bors
Copy link
Collaborator

bors commented Nov 9, 2023

☀️ Test successful - checks-actions
Approved by: albertlarsan68
Pushing 4c8862b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 9, 2023
@bors bors merged commit 4c8862b into rust-lang:master Nov 9, 2023
@rustbot rustbot added this to the 1.75.0 milestone Nov 9, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4c8862b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.8% [0.6%, 1.0%] 5
Regressions ❌
(secondary)
4.4% [4.4%, 4.4%] 1
Improvements ✅
(primary)
-0.7% [-1.2%, -0.4%] 7
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-1.2%, 1.0%] 12

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.7% [0.7%, 0.7%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.7% [0.7%, 0.7%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 662.476s -> 663.173s (0.11%)
Artifact size: 308.77 MiB -> 308.78 MiB (0.00%)

@tshepang tshepang deleted the pa-configure-git-diff branch November 10, 2023 06:25
celinval added a commit to celinval/rust-dev that referenced this pull request Jun 4, 2024
Update Rust toolchain from nightly-2023-11-09 to nightly-2023-11-10
without any other source changes.
This is an automatically generated pull request. If any of the CI checks
fail, manual intervention is required. In such a case, review the
changes at https://github.com/rust-lang/rust from
rust-lang@fdaaaf9
up to
rust-lang@0f44eb3.
The log for this commit range is:
rust-lang@0f44eb32f1 Auto merge of
rust-lang#117727 - saethlin:inline-derived-fmt, r=nnethercote
rust-lang@eae4135939 Auto merge of
rust-lang#117708 - onur-ozkan:x-setup, r=clubby789
rust-lang@4c8862b263 Auto merge of
rust-lang#117122 - ferrocene:pa-configure-git-diff, r=albertlarsan68
rust-lang@d32d9238cf Emit #[inline] on
derive(Debug)
rust-lang@b7583d38b7 Auto merge of
rust-lang#117712 - lcnr:expand-coroutine, r=jackh726
rust-lang@488dd9bc73 fmt
rust-lang@e7998aa21f Auto merge of
rust-lang#117734 - nnethercote:rm-Zstrip, r=davidtwco
rust-lang@287ae4db75 Auto merge of
rust-lang#117632 - Nilstrieb:icup, r=davidtwco
rust-lang@492e57c6ad Auto merge of
rust-lang#117736 - TaKO8Ki:rollup-fjrtmlb, r=TaKO8Ki
rust-lang@d1e26401bc chore(bootstrap):
capitalize {error, warning, info, note} tags
rust-lang@42fbf3ebf5 allow users to
override the existing configuration during x setup
rust-lang@3d6417fc7a check config file
before prompts on x setup
rust-lang@f5195c52bb Rollup merge of
rust-lang#117724 - Kobzol:shim-error-message, r=onur-ozkan
rust-lang@e603f4491c Rollup merge of
rust-lang#117723 - onur-ozkan:keep-bootstrap-on-x-clean, r=albertlarsan68
rust-lang@6533c62ce7 Rollup merge of
rust-lang#117705 - tshepang:patch-2, r=Nilstrieb
rust-lang@b4fa5b7004 Rollup merge of
rust-lang#117694 - jmillikin:core-io-borrowed-buf, r=m-ou-se
rust-lang@4cc549811f Rollup merge of
rust-lang#117645 - compiler-errors:auto-trait-subst, r=petrochenkov
rust-lang@a1a8d6fe9c Rollup merge of
rust-lang#116762 - WaffleLapkin:fixup_fromptr_docs, r=RalfJung
rust-lang@d8dbf7ca0e Auto merge of
rust-lang#117557 - Zoxc:panic-prio, r=petrochenkov
rust-lang@ecc936b155 Remove `-Z strip`.
rust-lang@57fb1e643a Auto merge of
rust-lang#117454 - shepmaster:github-actions-m1-tests,
r=GuillaumeGomez,onur-ozkan
rust-lang@341c85648c Move `BorrowedBuf`
and `BorrowedCursor` from `std:io` to `core::io`
rust-lang@92267c9794 update mir-opt tests
rust-lang@992d93f687 rename
`BorrowKind::Shallow` to `Fake`
rust-lang@a42eca42df generator layout:
ignore fake borrows
rust-lang@622be2d138 Restore rustc shim
error message
rust-lang@de0458af97 speed up `x clean`
rust-lang@6909992501 Run tests in CI for
aarch64-apple-darwin
rust-lang@64090536d4 Install tidy for
aarch64-apple-darwin
rust-lang@469d34b39b Mark Rustdoc test as
Linux-only
rust-lang@bf360d407e instrument
constituent types computation
rust-lang@03435e6fdd accept review
suggestion
rust-lang@769ad29c3e triagebot.toml: use
inclusive language
rust-lang@102384523a Document how rust
atomics work wrt mixed-sized and non-atomic accesses
rust-lang@c17d33f1df Extend builtin/auto
trait args with error when they have >1 argument
rust-lang@580fa0c1a9 rename
github_repository to git_repository
rust-lang@ffffc2038f Update ICU4X
rust-lang@ff1858e2aa Make
`FatalErrorMarker` lower priority than other panics
rust-lang@4a0873533f update suggest-tests
rust-lang@5a562d962e pass the correct
args to compiletest
rust-lang@545cc830e1 allow configuring
the parent GitHub repository

Co-authored-by: celinval <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants